Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper fix for iOS cropper being stuck #7194

Merged
merged 2 commits into from
Dec 19, 2024
Merged

Proper fix for iOS cropper being stuck #7194

merged 2 commits into from
Dec 19, 2024

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Dec 19, 2024

Reverts #7191 and adds a proper fix.

The root problem is hinted at in the error message in ivpusic/react-native-image-crop-picker#1631:

[Presentation] Attempt to present <TOCropViewController: 0x10d126a00> on <PHPickerViewController: 0x10be14b80> (from <PHPickerViewController: 0x10be14b80>) whose view is not in the window hierarchy.

We're trying to present the cropper from the picker but the picker is being dismissed — so it's not valid to present from. I'm not sure why this wasn't hitting us before but the problem makes sense.

The issue is how the cropper determines the controller to be presented from. It currently looks for the innermost controller, but it doesn't take it into account that the innermost controller might be animating out (and thus is not valid to present from). The fix is to add a condition for that.

Test Plan

Set an Xcode breakpoint on that line. Confirm that before the fix, it would select the picker controller as the "current" one, but after the fix, it selects the parent (as it should).

Verify all usages of the cropper:

  • Avi
  • Banner
  • Onboarding
fixx.mov

@arcalinea arcalinea temporarily deployed to proper-fix-ios-cropper - social-app PR #7194 December 19, 2024 21:39 — with Render Destroyed
Copy link

Old size New size Diff
6.84 MB 6.84 MB 0 B (0.00%)

@gaearon gaearon merged commit 10981ee into main Dec 19, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants